-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: integrate vitest matchers globally #3425
base: master
Are you sure you want to change the base?
chore: integrate vitest matchers globally #3425
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@starc007 is attempting to deploy a commit to the Fuel Labs Team on Vercel. A member of the Team first needs to authorize it. |
CodSpeed Performance ReportMerging #3425 will improve performances by 47.93%Comparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @starc007,
Thank you so much for this contribution. We really appreciate it 🙏
It appears that the current solution hasn’t resolved the TS errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR also needs a changeset, please run pnpm changeset
from the root directory.
5b0f72a
to
b70acc1
Compare
Hey @Torres-ssf Issue: This suggests the Questions:
Any guidance on the correct way to set this up would be appreciated. |
}, | ||
"include": ["src", "test"] | ||
"include": ["src", "global.d.ts"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"include": ["src", "global.d.ts"] | |
"include": ["src", "test", "global.d.ts"] |
We want to include the test
directory still.
interface Matchers<R = BN> { | ||
toEqualBn: (expected: BNInput) => R; | ||
} | ||
|
||
const pass = received.eq(argument); | ||
|
||
if (pass) { | ||
return { | ||
message: () => `Expected ${received.toString()} not to equal ${argument.toString()}`, | ||
pass: true, | ||
}; | ||
declare module 'vitest' { | ||
interface Assertion extends Matchers {} | ||
interface AsymmetricMatchersContaining extends Matchers {} | ||
interface ExpectStatic { | ||
toEqualBn(expected: BNInput): BN; | ||
} | ||
return { | ||
message: () => `expected ${received.toString()} to equal ${argument.toString()}`, | ||
pass: false, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Replace the contents of
packages/fuel-gauge/test/global.d.ts
with this. - Add the following to the
tsconfig.test.json
git cherry-pick 8ed2d14d5f57581746b260b4d83364af3a01c479
.
I believe this should resolve the linting issue.
import { setupTestMatchers } from './src/abi/vitest.matcher'; | ||
|
||
setupTestMatchers(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move the contents of vitest.matcher
to this file?
@@ -0,0 +1,5 @@ | |||
--- | |||
"fuels": minor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"fuels": minor |
There are no changes to fuels
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this won't live in fuel-gauge
which is our integration testing directory, vitest config should live outside here and be usable throughout the SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @danielbate. These new matchers should be accessible for use in tests across any package.
The issue is that the created matchers rely on types imported from the math package. Ideally, these matchers should not depend on imported types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could move the setup to packages/utils/src/test-utils
as @fuel-ts/utils
is available at the root level as a devDependency
.
It could also be helpful to end consumers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yaa that location makes sense since @fuel-ts/utils is available as a root devDependency and would make these matchers accessible across packages.
let me know if we want to go this way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go for it @starc007 - makes sense to me :)
Will convert to draft until you're ready.
vitest
matchers globally #3421Summary
This PR integrates Vitest matchers globally for fuel-gauge tests, improving test maintainability and type safety.
Changes
test/setup.ts
toEqualBn
expect.extend()
calls from test files@ts-expect-error
comments fortoEqualBn
usageTesting